Fix: close client session to avoid resource leak#70
Fix: close client session to avoid resource leak#70guyskk wants to merge 6 commits intocuenca-mx:mainfrom
Conversation
|
Hi @matin, any feedbacks on the pull request? |
|
@guyskk thanks for the contribution! This approach would definitely work, but what about the following approach instead? from datetime import date
from cep import Transferencia
tr_details = dict(
fecha=date(2019, 4, 12),
clave_rastreo='CUENCA1555093850',
emisor='90646', # STP
receptor='40012', # BBVA
cuenta='012180004643051249',
monto=8.17,
)
with Transferencia.validar(**tr_details) as tr:
pdf = tr.descargar()Using What do you think? |
|
@matin The problem is If we can move with TransferenciaClient() as client:
tr = client.validar(**tr_details)
pdf = tr.descargar()But it may be a break change, or keep the old api, only deprecate them. |
|
Hi @matin, I implemented transferencia client use context manager, and reserved all existing api, only deprecated them in docs. Could you help review again? |
Looks good to me. @rogelioLpz can you review as well? |
|
@guyskk can you bump the version as well? |
|
@matin bumped version from 0.1.7 to 0.2.0, increased minor version because the PR add new APIs. |
|
Leaving to @rogelioLpz for final review |
|
Hi @guyskk and thanks for the contribution! Please apply your changes in |
|
Hi @rogelioLpz , the Transferencia.validar may return None, which cause not able to use context manager, so we have to add another class. #70 (comment) |
|
Hi @rogelioLpz , any suggestions or feedbacks on the PR? |
Fix requests session not closed which will cause resource leak.